Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Makefile: Install and library targets and configurable BUILDDIR #2

Merged
merged 7 commits into from
Nov 8, 2023

Conversation

zuiderkwast
Copy link
Collaborator

@zuiderkwast zuiderkwast commented Nov 2, 2023

  • Configurable BUILDDIR
  • Add install and library targets
  • Remove unnecessary variables

Makefile Outdated Show resolved Hide resolved
Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zuiderkwast zuiderkwast changed the title Makefile: Add install and library targets and configurable BINDIR Makefile: Install and library targets and configurable BUILDDIR Nov 6, 2023
Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

WDYT about renaming SOLIBNAME to LIBNAME, and SOLIBNAME to LIBNAME?
i.e

LIBNAME = liburl_parser
SONAME ?= $(LIBNAME).$(SOEXT).$(SOMAJOR).$(SOMINOR)
SOLIBNAME= $(LIBNAME).$(SOEXT).$(SOMAJOR).$(SOMINOR).$(SOREV)

Then the LIBNAME can additionally be used for the static library.. or maybe we divert to much from the original then?

@zuiderkwast
Copy link
Collaborator Author

WDYT about renaming SOLIBNAME to LIBNAME, and SOLIBNAME to LIBNAME?

Yes we can. It makes sense.

or maybe we divert to much from the original then?

Yes, that's why i didn't do the change. But it doesn't matter much. It's more important than the C code is as close as possible to the original. I'll do the change.

@zuiderkwast
Copy link
Collaborator Author

Looks better? Do you want a variable for $(LIBNAME).a?

Copy link
Collaborator

@bjosv bjosv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy!

@zuiderkwast zuiderkwast merged commit 33a7008 into Nordix:main Nov 8, 2023
1 check passed
@zuiderkwast zuiderkwast deleted the lib-and-install-targets branch November 8, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants